Skip to content

feat: add subject filter with sessionStorage persistence #25

Jump to bottom
Merged
merged 2 commits into from
Sep 9, 2026
Merged

feat: add subject filter with sessionStorage persistence #25

merged 2 commits into from
Sep 9, 2026
+283 −7

Conversation

robertky
Copy link
Member

@robertky robertky commented Sep 9, 2026 •
edited
Loading

Summary

Closes #19.

  • Adds src/components/SubjectFilter.tsx: a props-driven, accessible <select> with a visible label and six supported subjects
  • Adds src/components/SubjectFilter.css: responsive styling and visible keyboard focus
  • Adds src/constants/subjects.ts: the shared subject list and default subject
  • Adds src/hooks/usePreferences.ts: reads and writes t19.preferences in sessionStorage, preserves future preferences, and safely falls back to Fantasy when data is missing, invalid, or unsupported
  • Updates src/App.tsx: the chosen subject now drives useBooks(subject), so changing the selector fetches that subject’s books; the viewer continues to start with the first result

User flow

  1. Choose a subject from the selector.
  2. The app loads books for that subject.
  3. Refresh the page: the selected subject remains chosen for the current browser session.
  4. Invalid stored data safely returns to Fantasy.

Tests

  • SubjectFilter renders all options, reports a changed selection, and has snapshot coverage
  • App test verifies a subject change makes exactly one additional mocked OpenLibrary request and stores the selection
  • App tests verify saved and corrupt sessionStorage values
  • App snapshot updated deliberately for the new selector
  • Tests use MSW only; no real network calls

Validation

  • npm run format:check
  • npm run lint
  • npm test — 29 passed
  • npm run build

@robertky robertky requested a review from a team as a code owner September 9, 2026 12:17
@robertky robertky added status: in-review PR is open and awaiting review ai-assisted Substantial AI-generated code (documented per spec) labels Sep 9, 2026
@erikhfj
Copy link
Member

erikhfj commented Sep 9, 2026

Code review pass (automated, thorough): no bugs found. All gates verified locally on the branch (format ✓, lint 0 warnings ✓, 29/29 tests ✓, build ✓) and it merges cleanly with current main.

Specifically cleared: usePreferences storage robustness (corrupt JSON, null, arrays, primitives all fall back safely; try/catch covers privacy-mode/quota), per-subject caching via ['books', subject] (test clients use staleTime: 0 so request-count assertions stay deterministic; prod staleTime comes from main.tsx), encodeURIComponent in the api layer, and a11y (label association, focus outline).

Three non-blocking notes:

  1. Low — write-side validation: setSubject persists any string; the invariant (must be in SUBJECT_OPTIONS) is enforced only on read. No current impact since the select is the only caller, and read-side fallback self-heals.
  2. Info — UX during subject change: with no placeholderData, the displayed book clears while the new subject loads (shows the loading state). Looks intentional; flag for the feat: useBooks hook with TanStack Query and App integration #12 wiring so the book index resets on subject change rather than pointing into the old list.
  3. Checklist: snapshots were updated/added — make sure the PR description states that per our convention.

Ready to merge from my perspective.

@erikhfj erikhfj mentioned this pull request Sep 9, 2026
Merged
4 tasks
@erikhfj erikhfj merged commit 30637ae into main Sep 9, 2026
1 check passed
@erikhfj erikhfj deleted the feat/19-subject-filter branch September 9, 2026 16:07
@erikhfj erikhfj mentioned this pull request Sep 9, 2026
Merged
4 tasks
Sign in to join this conversation on GitHub.
Reviewers

@erikhfj erikhfj erikhfj approved these changes

Assignees
No one assigned
Labels
ai-assisted Substantial AI-generated code (documented per spec) status: in-review PR is open and awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: Subject filter with sessionStorage persistence
2 participants
@robertky @erikhfj